Skip to content

fix(diagnostics): enforce current-editor snapshots and stale results.#95

Merged
knightedcodemonkey merged 2 commits intonextfrom
bananas
Apr 24, 2026
Merged

fix(diagnostics): enforce current-editor snapshots and stale results.#95
knightedcodemonkey merged 2 commits intonextfrom
bananas

Conversation

@knightedcodemonkey
Copy link
Copy Markdown
Owner

Copilot AI review requested due to automatic review settings April 24, 2026 02:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses diagnostics race conditions (#93) by making lint/typecheck runs commit results only when they match the current editor/tab snapshot, and updates the diagnostics drawer to better reflect multi-tab workflows.

Changes:

  • Add run identity + source version guards to prevent stale typecheck and lint completions from overwriting newer state.
  • Capture current editor/tab snapshots when triggering diagnostics to ensure runs always evaluate the intended source/path.
  • Update diagnostics drawer layout/labels for styles vs component tabs and extend Playwright coverage for stale-result scenarios.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/modules/diagnostics/type-diagnostics.js Adds sourceVersion + workspace tab snapshot overrides to ignore late typecheck results and stabilize auto-recheck timing.
src/modules/diagnostics/lint-diagnostics.js Adds runContext + isRunContextCurrent commit guards and exposes targeted cancel methods.
src/modules/app-core/workspace-tab-selection-controller.js Adds onActiveWorkspaceTabChange callback to react to tab selections.
src/modules/app-core/workspace-controllers-setup.js Wires onActiveWorkspaceTabChange into tab selection controller setup.
src/modules/app-core/diagnostics-tab-state-helpers.js New helper for drawer layout sync, tab-switch clearing, and lint target identity resolution.
src/modules/app-core/diagnostics-flow-controller.js Adds lint run context identity/versioning and cancels lint runs on staleness; adjusts drawer/status behavior.
src/modules/app-core/app-composition-options.js Plumbs lint target getters into runtime core options.
src/modules/app-core/app-bindings-startup.js Ensures diagnostics run off persisted active-tab snapshots; adjusts reset behavior and initial drawer layout sync.
src/app.js Wires new diagnostics helpers, stores flow controller reference, and plumbs snapshot + tab-kind helpers.
playwright/diagnostics.spec.ts Updates reset button expectations and adds regression tests for stale results + tab-driven drawer layout.
Comments suppressed due to low confidence (2)

src/modules/app-core/diagnostics-flow-controller.js:196

  • scheduledComponentLintRecheck / scheduledStylesLintRecheck and their clear helpers remain, but the PR removed all code paths that ever assign these timers. This leaves dead state + branches that are now misleading. Either remove the unused timer variables/helpers, or reintroduce the scheduling logic if auto re-check is still intended for lint.
  let scheduledComponentLintRecheck = null
  let scheduledStylesLintRecheck = null
  let componentLintPending = false
  let stylesLintPending = false
  let componentLintSourceVersion = 0
  let stylesLintSourceVersion = 0

  const normalizeLintTargetIdentity = target => {
    const tabId = typeof target?.tabId === 'string' ? target.tabId.trim() : ''
    const path = typeof target?.path === 'string' ? target.path.trim() : ''
    const language =
      typeof target?.language === 'string' ? target.language.trim().toLowerCase() : ''

    return {
      tabId,
      path,
      language,
      key: `${tabId}|${path}|${language}`,
    }
  }

  const getCurrentLintTargetIdentity = scope => {
    const resolveTarget =
      scope === 'styles' ? getStylesLintTarget : getComponentLintTarget
    return normalizeLintTargetIdentity(resolveTarget())
  }

  const createLintRunContext = scope => {
    const target = getCurrentLintTargetIdentity(scope)
    const sourceVersion =
      scope === 'styles' ? stylesLintSourceVersion : componentLintSourceVersion

    return {
      scope,
      sourceVersion,
      targetKey: target.key,
    }
  }

  const isLintRunContextCurrent = (scope, runContext) => {
    if (!runContext || runContext.scope !== scope) {
      return false
    }

    const currentSourceVersion =
      scope === 'styles' ? stylesLintSourceVersion : componentLintSourceVersion
    if (runContext.sourceVersion !== currentSourceVersion) {
      return false
    }

    const currentTarget = getCurrentLintTargetIdentity(scope)
    return runContext.targetKey === currentTarget.key
  }

  const setTypecheckButtonLoading = isLoading => {
    const runtimeTypecheckButton = document.getElementById('typecheck-button')
    if (!(runtimeTypecheckButton instanceof HTMLButtonElement)) {
      return
    }

    runtimeTypecheckButton.classList.toggle('render-button--loading', isLoading)
    runtimeTypecheckButton.setAttribute('aria-busy', isLoading ? 'true' : 'false')
    runtimeTypecheckButton.disabled = isLoading
  }

  const setLintButtonLoading = ({ button, isLoading }) => {
    if (!(button instanceof HTMLButtonElement)) {
      return
    }

    button.classList.toggle('render-button--loading', isLoading)
    button.setAttribute('aria-busy', isLoading ? 'true' : 'false')
    button.disabled = isLoading
  }

  const typeDiagnostics = createTypeDiagnosticsController({
    cdnImports,
    importFromCdnWithFallback,
    getTypeScriptLibUrls,
    getTypePackageFileUrls,
    getJsxSource,
    getTypecheckSourcePath,
    getWorkspaceTabs,
    getRenderMode,
    setTypecheckButtonLoading,
    setTypeDiagnosticsDetails,
    setTypeDiagnosticsPending,
    setStatus,
    setRenderedStatus: () => {
      if (typeDiagnostics.getLastTypeErrorCount() > 0) {
        setStatus(
          `Rendered (Type errors: ${typeDiagnostics.getLastTypeErrorCount()})`,
          'error',
        )
        return
      }

      if (statusNode.textContent.startsWith('Rendered (Type errors:')) {
        setStatus('Rendered', 'neutral')
      }
    },
    isRenderedStatus: () =>
      statusNode.textContent === 'Rendered' ||
      statusNode.textContent.startsWith('Rendered (Type errors:'),
    isRenderedTypeErrorStatus: () =>
      statusNode.textContent.startsWith('Rendered (Type errors:'),
    incrementTypeDiagnosticsRuns,
    decrementTypeDiagnosticsRuns,
    getActiveTypeDiagnosticsRuns,
    onIssuesDetected: ({ issueCount }) => {
      if (issueCount > 0) {
        setDiagnosticsDrawerOpen(true)
      }
    },
  })

  const setRenderedStatus = () => {
    if (typeDiagnostics.getLastTypeErrorCount() > 0) {
      setStatus(
        `Rendered (Type errors: ${typeDiagnostics.getLastTypeErrorCount()})`,
        'error',
      )
      return
    }

    if (statusNode.textContent.startsWith('Rendered (Type errors:')) {
      setStatus('Rendered', 'neutral')
    }
  }

  const lintDiagnostics = createLintDiagnosticsController({
    cdnImports,
    importFromCdnWithFallback,
    getComponentSource: getJsxSource,
    getStylesSource: getCssSource,
    getStyleMode,
    setComponentDiagnostics: setStyleDiagnosticsDetails,
    setStyleDiagnostics: setStyleDiagnosticsDetails,
    setStatus,
    onIssuesDetected: ({ issueCount }) => {
      if (issueCount > 0) {
        setDiagnosticsDrawerOpen(true)
      }
    },
  })

  const clearComponentLintRecheckTimer = () => {
    if (scheduledComponentLintRecheck) {
      clearTimeout(scheduledComponentLintRecheck)
      scheduledComponentLintRecheck = null
    }
  }

  const clearStylesLintRecheckTimer = () => {
    if (scheduledStylesLintRecheck) {
      clearTimeout(scheduledStylesLintRecheck)
      scheduledStylesLintRecheck = null
    }
  }

src/modules/app-core/diagnostics-flow-controller.js:327

  • The updated lint staleness behavior no longer schedules an automatic re-check when unresolved lint issues exist; it always marks lint as stale and requires a manual rerun. This contradicts the issue/#93 requirements to preserve the auto re-check-after-edit behavior when unresolved issues are present. Consider restoring a per-scope hasUnresolvedLintIssues/lastLintIssueCount + debounce scheduling (with run identity + sourceVersion/target guards), and only switch to manual rerun when there were no unresolved issues.
  const markComponentLintDiagnosticsStale = () => {
    componentLintSourceVersion += 1
    clearComponentLintRecheckTimer()

    activeComponentLintAbortController?.abort()
    activeComponentLintAbortController = null
    lintDiagnostics.cancelComponent()
    setLintButtonLoading({ button: lintComponentButton, isLoading: false })

    componentLintPending = false
    syncLintPendingState()
    setStyleDiagnosticsDetails({
      headline: 'Source changed. Click Lint to run diagnostics.',
      level: 'muted',
    })

    if (statusNode.textContent.startsWith('Rendered (Lint issues:')) {
      setStatus('Rendered', 'neutral')
    }
  }

  const markStylesLintDiagnosticsStale = () => {
    stylesLintSourceVersion += 1
    clearStylesLintRecheckTimer()

    activeStylesLintAbortController?.abort()
    activeStylesLintAbortController = null
    lintDiagnostics.cancelStyles()
    setLintButtonLoading({ button: lintStylesButton, isLoading: false })

    stylesLintPending = false
    syncLintPendingState()
    setStyleDiagnosticsDetails({
      headline: 'Source changed. Click Lint to run diagnostics.',
      level: 'muted',
    })

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/modules/app-core/diagnostics-flow-controller.js
Comment thread src/modules/app-core/diagnostics-tab-state-helpers.js Outdated
Comment thread src/modules/app-core/diagnostics-tab-state-helpers.js
Comment thread src/modules/app-core/app-bindings-startup.js Outdated
@knightedcodemonkey knightedcodemonkey merged commit 2258ab8 into next Apr 24, 2026
6 checks passed
@knightedcodemonkey knightedcodemonkey deleted the bananas branch April 24, 2026 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants